Skip to content

Fix link cycles with non-existent paths#148

Merged
wagoodman merged 1 commit intomainfrom
fix-dead-link-cycles
Nov 30, 2022
Merged

Fix link cycles with non-existent paths#148
wagoodman merged 1 commit intomainfrom
fix-dead-link-cycles

Conversation

@wagoodman
Copy link
Copy Markdown
Contributor

@wagoodman wagoodman commented Nov 30, 2022

The filetree does not handle cycles with symlinks well when the file nodes do not exist:

$ ln -s  noobaa-core/../acorn/bin/acorn acorn
$ ls -al
total 0
drwxr-xr-x    3 wagoodman  staff    96 Nov 30 09:49 .
drwxr-xr-x  118 wagoodman  staff  3776 Nov 30 09:49 ..
lrwxr-xr-x    1 wagoodman  staff    30 Nov 30 09:49 acorn -> noobaa-core/../acorn/bin/acorn
$ syft .
⠼ Indexing .              [file: /Users/wagoodman/scratch/deadlinkloop/acorn]runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc021140420 stack=[0xc021140000, 0xc041140000]
fatal error: stack overflow

runtime stack:
runtime.throw({0x303cf64?, 0x4529300?})
        /opt/hostedtoolcache/go/1.18.8/x64/src/runtime/panic.go:992 +0x71
runtime.newstack()
        /opt/hostedtoolcache/go/1.18.8/x64/src/runtime/stack.go:1101 +0x5cc
runtime.morestack()
        /opt/hostedtoolcache/go/1.18.8/x64/src/runtime/asm_amd64.s:547 +0x8b

goroutine 98 [running]:
runtime.heapBitsSetType(0xc00e17db80?, 0x80?, 0x80?, 0x2bf9360?)
        /opt/hostedtoolcache/go/1.18.8/x64/src/runtime/mbitmap.go:832 +0xbcc fp=0xc021140430 sp=0xc021140428 pc=0x10158ec
runtime.mallocgc(0x80, 0x2bf9360, 0x1)
        /opt/hostedtoolcache/go/1.18.8/x64/src/runtime/malloc.go:1117 +0x673 fp=0xc0211404a8 sp=0xc021140430 pc=0x100d073
runtime.makeslice(0xc00109a100?, 0x33daaa8?, 0x33daaa8?)
        /opt/hostedtoolcache/go/1.18.8/x64/src/runtime/slice.go:103 +0x52 fp=0xc0211404d0 sp=0xc0211404a8 pc=0x104c332
strings.genSplit({0xc00109a100, 0x35}, {0x33daaa8, 0x1}, 0x0, 0x11b1f798?)
        /opt/hostedtoolcache/go/1.18.8/x64/src/strings/strings.go:247 +0x6d fp=0xc021140530 sp=0xc0211404d0 pc=0x1100b8d
strings.Split(...)
        /opt/hostedtoolcache/go/1.18.8/x64/src/strings/strings.go:303
github.com/anchore/stereoscope/pkg/filetree.(*FileTree).resolveAncestorLinks(0x2e0f200?, {0xc00109a100, 0x35})
        /home/runner/go/pkg/mod/github.com/anchore/stereoscope@v0.0.0-20221006201143-d24c9d626b33/pkg/filetree/filetree.go:206 +0x85 fp=0xc0211405c0 sp=0xc021140530 pc=0x2611d85
github.com/anchore/stereoscope/pkg/filetree.(*FileTree).resolveNodeLinks(0xc0010b6020?, 0xc0010983c0, 0x1)
        /home/runner/go/pkg/mod/github.com/anchore/stereoscope@v0.0.0-20221006201143-
...
<goes on for a while...>

This PR fixes this behavior by providing a shared state between resolveNodeLinks and resolveAncestorLinks calls, short circuiting the link resolution when a non-existent node is provided twice in the search.

Addresses anchore/syft#1368

@wagoodman
fix link cycles with non-existent paths
a545a37 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman wagoodman requested a review from a team November 30, 2022 15:23
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 30, 2022

Benchmark Test Results

Benchmark results from the latest changes vs base branch 
name                                                time/op
pkg:github.com/anchore/stereoscope/pkg/file goos:linux goarch:amd64
TarIndex-2                                          42.7µs ± 0%
pkg:github.com/anchore/stereoscope/test/integration goos:linux goarch:amd64
SimpleImage_GetImage/docker-archive-2               1.43ms ± 1%
SimpleImage_GetImage/oci-archive-2                  1.10ms ± 6%
SimpleImage_GetImage/oci-dir-2                       727µs ± 4%
SimpleImage_FetchSquashedContents/docker-archive-2  17.6µs ± 9%

name                                                alloc/op
pkg:github.com/anchore/stereoscope/pkg/file goos:linux goarch:amd64
TarIndex-2                                          5.69kB ± 0%
pkg:github.com/anchore/stereoscope/test/integration goos:linux goarch:amd64
SimpleImage_GetImage/docker-archive-2                358kB ± 0%
SimpleImage_GetImage/oci-archive-2                   641kB ± 0%
SimpleImage_GetImage/oci-dir-2                       403kB ± 0%
SimpleImage_FetchSquashedContents/docker-archive-2  2.71kB ± 0%

name                                                allocs/op
pkg:github.com/anchore/stereoscope/pkg/file goos:linux goarch:amd64
TarIndex-2                                            93.0 ± 0%
pkg:github.com/anchore/stereoscope/test/integration goos:linux goarch:amd64
SimpleImage_GetImage/docker-archive-2                2.67k ± 0%
SimpleImage_GetImage/oci-archive-2                   1.44k ± 0%
SimpleImage_GetImage/oci-dir-2                       1.23k ± 0%
SimpleImage_FetchSquashedContents/docker-archive-2    21.0 ± 0%

@wagoodman wagoodman merged commit e38c6f1 into main Nov 30, 2022
@wagoodman wagoodman deleted the fix-dead-link-cycles branch November 30, 2022 15:30
@wagoodman wagoodman mentioned this pull request Nov 30, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kzantow kzantow kzantow approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wagoodman @kzantow